Skip to content

fix: REST-created claude panes get preallocated session identity (parity with WS lane) — fixes A13 blindness - #584

Merged
danshapiro merged 16 commits into
mainfrom
fix/rest-terminal-session-identity
Jul 30, 2026
Merged

fix: REST-created claude panes get preallocated session identity (parity with WS lane) — fixes A13 blindness#584
danshapiro merged 16 commits into
mainfrom
fix/rest-terminal-session-identity

Conversation

@danshapiro

Copy link
Copy Markdown
Owner

Bug Summary

REST-created claude panes (POST /api/tabs mode=claude) never acquired session identity — no preallocated --session-id, sessionRef stayed null forever. This left them un-resumable and, worse, blinded the A13 live-owner guard: a /resume of a session live inside an unbound REST pane was not refused.

Issue (kata hbsa): REST path split from WS path early in the lifecycle, skipping the preallocated session-id minting that the WS fresh-claude path does.

Live-Drill Evidence

Post-deploy drill on production (commit 4c04dc9) verified:

  • WS fresh-claude path (known working): binds in ~400ms via preallocated --session-id
  • A13 guard verified: refuses cross-pane resume when live session is bound

Vulnerability confirmed:

  • Opened REST POST /api/tabs mode=claude
  • Attempted /resume of that session from another pane
  • A13 guard did not refuse (session identity was null, so the guard couldn't prove ownership)

Fix Shape

Brought REST create path to parity with WS fresh-claude path:

  1. Shared preallocation predicate used by both paths
  2. REST session-id minting surfaced to clients via response
  3. Binder seam with ledger-backed pre-spawn binding (PIN 2: durability-before-argv)
  4. Full pipeline threading from create request → spawn → ledger persistence
  5. Regression pins:
    • Identity survives signal destruction
    • A13 refuses cross-pane resume of a live REST-pane session
    • SessionStart signal consumed not retained

Test Coverage

  • Workspace tests (post-merge): 333 pass, 1 flaky file-lock (pre-existing infrastructure issue, not code)
  • New tests: rest_claude_identity.rs, rest_locator_identity.rs — identity lifecycle regression pins
  • Regression suite: A13 guard, ledger persistence, signal consumption

Implementation Notes

Main merge: Branch based on commit 4c04dc9; origin/main advanced with #582 (znhn-bccd-followups) and #583 (resume-session-button). Merge resolved cleanly; post-merge fixes applied to test initializers for new auto_resume_cancels field.

Transparency note (per session instructions): The implementation plan's independent review hit its 3-round iteration cap without a recorded clean pass — all blocking findings from every round were fixed, and the final code passed independent whole-branch review and delta review with zero blocking issues.

References: Kata hbsa, addresses A13 live-owner guard blindness

Generated with Amplifier

danshapiro and others added 16 commits July 29, 2026 16:45
…entity plan

Stage-2 validation (12 verified, 2 falsified) of kata hbsa's plan:
- A1 falsified: predicate feed must use the parsed sessionRef locator
  presence (JSON null is absent on both doors) — Task 2 fixed, null-mint
  regression pin added
- A2 falsified: never-retired REST identity rows leave dead panes
  live-looking (session directory, rename cascade, late-signal rebind) —
  PaneIdentityBinder gains retire_pane_identity wired into the REST exit
  hook, with unit + e2e pins
- A3 caveat: failure-delete must precede the AlreadyExists 409 early-return
- A8: main.rs wiring corrected (PaneLedger constructed at :527 — hoist it)
- A12: entry-point audit settled (tabs-sync/restore deliberately deleted,
  kata h9vt); coverage map updated
- A5/A11 validation notes folded into Tasks 2-3

Full ledger: .the-usual-logs/rest-terminal-session-identity/load-bearing-ledger.md
…nc-trait dep, tracked superpowers plan premise

Fresh-eyes review iteration 1 found three blocking defects in the plan:

1. The plan claimed docs/superpowers/plans/2026-07-29-rest-terminal-session-identity.md
   was untracked and Task 8's final gate expected git status to show it
   untracked — but that file is committed on this branch (eb28b3d).
   Rewrote the Global Constraint and the Task 8 Step 4 gate to match the
   actual repo state (status must be empty).

2. Task 4 instructed copying an async-trait dependency line from
   freshell-freshagent/Cargo.toml — no crate in the workspace depends on
   async-trait (the house pattern is boxed-future aliases). Removed the
   dependency entirely.

3. Task 5 Step 4.5 instructed .await on retire_pane_identity inside the
   REST exit hook — ExitHook is a sync FnOnce on the PTY reader OS thread
   with no tokio runtime, so that step could not compile/run as written.

Joint fix for 2+3: PaneIdentityBinder is now a fully synchronous trait
(the SessionIdentityLookup precedent — every underlying registry/ledger
operation is sync). Async REST call sites hop ledger-touching calls
through awaited tokio::task::spawn_blocking (the WS create path's own
idiom, terminal.rs:2211-2234), preserving PIN 2 ordering; the exit hook
calls retire_pane_identity inline sync, exactly mirroring the WS exit
hook (terminal.rs:1334-1342). Updated trait/impl/test snippets, fake
binder, call-site snippets, file tables, and commit staging lists
accordingly (no Cargo.toml/Cargo.lock changes remain in the plan).
… surfaces (broadcast paneContent + registry rows, not REST HTTP bodies); spot-revert the mint gate, not launch_intent
… mode:claude (respawn is body-driven, empty body respawns shell); retire_pane_identity mirrors the natural-exit hook only (no retire_closed, Bound row survives natural exit)
…oth create doors

🤖 Generated with Amplifier

Co-Authored-By: Amplifier <[email protected]>
… (WS parity, kata hbsa)

Fresh POST /api/tabs {mode:"claude"} now mints a server-preallocated
session UUID via the shared freshell_platform::should_preallocate_fresh_claude
predicate (Task 1), flips the CLI launch intent to LaunchIntent::Start so
`claude --session-id <uuid>` lands in argv, and threads a
claude_fresh_prealloc flag through GatedSettleInputs for Task 5's
PIN 2-gated binder call sites (eaa25b7 symmetry).

derive_resume_identity additionally returns the pre-provider-filter
locator parse result (ledger A1: "sessionRef": null must still mint,
matching the WS door's serde Option semantics). The
tab_create_missing_session_identity warn gains an explicit
paneContent.sessionRef guard — a create that ended up with real identity
must not alarm.

Minted ids claim no D7/D8 sessionRef lease (accepted_session_ref stays
None), and REST HTTP bodies are unchanged — sessionRef surfaces on the
broadcast ui.command paneContent and the registry row.

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <[email protected]>
…er bridge for the REST lane

Task 4 of the REST terminal session-identity plan (kata hbsa): a synchronous
PaneIdentityBinder trait in freshell-terminal (SessionIdentityLookup's
write-side twin) and its production impl LedgerPaneIdentityBinder in
freshell-ws, mirroring the WS fresh-claude path's registry+ledger writes
(PIN2 pre-spawn binding + failure-delete, post-spawn identity/binding/
pending block) and the exit hook's retire + pending-delete hygiene.
Ledger writes are best-effort: warn and proceed, never block a create.

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <[email protected]>
…s via PaneIdentityBinder (kata hbsa)

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <[email protected]>
…REST-live sessions, signal consumption (kata hbsa)

🤖 Generated with [Amplifier](https://github.com/microsoft/amplifier)

Co-Authored-By: Amplifier <[email protected]>
@danshapiro
danshapiro merged commit 6147c70 into main Jul 30, 2026
3 checks passed
pull Bot pushed a commit to HinchK/freshell that referenced this pull request Jul 31, 2026
…erminal identity (danshapiro#584), deck visual tweaks (danshapiro#585)

Conflict resolution (crates/freshell-freshagent/src/terminal_tabs.rs,
settle_gated_create): kept BOTH sides, ordered spawn-gate acquire (D-C-R,
ours) BEFORE the PIN2 claude prespawn durable binding (theirs) — a gate
rejection must not leave a stale prespawn ledger row, so the durable write
happens only after a permit is secured.

Post-merge semantic fix: pinned FRESHELL_CODEX_MANAGED_LAUNCH=0 in
rest_locator_identity.rs::rest_created_codex_pane_binds_identity_row_and_ledger
(new in danshapiro#584, written pre-flip; exercises the plain-CLI codex path with a
sleeper fake that cannot serve an app-server sidecar).

Pre-merge analysis recorded no redundancy between the danshapiro#582 review-hardening
(d9705fd) and the remote delta; danshapiro#586 touches resume-resolve-contract.ts,
not session-contract.ts — CrashTrace work composes cleanly.

Generated with Amplifier

Co-Authored-By: Amplifier <[email protected]>
pull Bot pushed a commit to HinchK/freshell that referenced this pull request Jul 31, 2026
…launch, danshapiro#582 breaker, danshapiro#584 identity)

Fold main's post-fork struct/initializer fields (auto_resume_cancels,
resume_cycles, HubConfig/CrashContext breaker fields) into branch-added
harnesses and literals after replaying onto origin/main.
pull Bot pushed a commit to HinchK/freshell that referenced this pull request Jul 31, 2026
Main danshapiro#584 added should_preallocate_fresh_claude keyed on 'no resume id';
a gate-fired claude fallback carries a minted id and silently skipped the
pre-spawn ledger binding. Thread claude_fresh_prealloc through
RestResumeOutcome and OR it into the prealloc decision.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant